Introduce AutoMonitor for simpler enablement of Application Signals #297

sky333999 · 2025-03-24T00:21:19Z

Description of changes: Introduce Auto Monitor for Application Signals on K8s which provides an easier onboarding experience. When enabled, the Operator can auto-instrument K8s workloads that are associated with K8s services as they come up automatically without requiring explicit opt-in via workload specific annotations.

The operator now accepts a new arg auto-monitor-config that has the following structure:

{
  "monitorAllServices": false,
  "languages": [],
  "restartPods": false,
  "customSelector": {
    "java": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "python": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "dotnet": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "nodejs": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    }
  },
  "exclude": {
    "java": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "python": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "dotnet": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    },
    "nodejs": {
      "namespaces": [], "deployments": [], "daemonsets": [], "statefulsets": []
    }
  }
}

monitorAllServices: Enable AutoMonitor for all services in the cluster i.e. for any K8s workloads that are brought up in the cluster, auto-instrument them. Defaults to false i.e. not enabled.
langugages: Specify the list of languages that AutoMonitor should inject auto-instrumentation for. Defaults to all supported languages.
restartPods: When enabled, restart pods that are already running on the cluster corresponding to K8s workloads that now fall under the scope of AutoMonitor. Defaults to false i.e. not enabled.
customSelector: Provides more granular control to explicitly specify namespaces and workloads that should be auto-instrumented. Can be specified even if monitorAllServices is disabled.
exclude: Similar to customSelector, but is instead used to deny specific namespaces or workloads from getting auto-instrumented when monitorAllServices is enabled. Takes precedence over customSelector in the case of conflicts.

Additional Notes:

customSelector with restartPods enabled serves as a replacement for the existing auto-annotation-config arg. auto-annotation-config will be deprecated entirely in a future release. Using both is consider invalid and will continue to respect auto-annotation-config in such scenarios.
monitorAllServices by default does not auto-instrument services in the kube-system and amazon-cloudwatch namespaces. If you require services in these namespaces to be auto-instrumented, you can explicitly specify them using customSelector.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…plate ObjectMeta

…h AutoMonitor and AnnotationMutators, check if safe to mutate before applying custom selector, run opt out tests for each type of workload, test opt out by disabling monitorallservices

CustomSelectors should obey autoRestart, autoAnnotateAutoInstrumentation shouldn't. Rename MonitorInterface to InstrumentationAnnotator. Add docs.

main.go

…estartPods

…ntation container validation by default. This is required for automonitor to work, because it tries to apply all init containers to each selected pod.

… update pod templates in workloads.

…el custom selector should not update pod template

…art namespace for autoAnnotateAutoInstrumentation.

integration-tests/util/util.go

pkg/instrumentation/annotationtype.go

pkg/instrumentation/auto/callback.go

pkg/instrumentation/auto/config.go

pkg/instrumentation/auto/monitor.go

pkg/instrumentation/podmutator.go

internal/webhook/namespacemutation/webhookhandler.go

main.go

lisguo · 2025-05-07T21:29:17Z

main.go

+		if err := json.Unmarshal([]byte(autoAnnotationConfigStr), &autoAnnotationConfig); err != nil {
+			setupLog.Error(err, "Unable to unmarshal auto-annotation config")
+		} else {
+			// todo: technically a breaking change, because previously an empty autoAnnotationConfig would clear all annotations, but automonitor does not reproduce this behavior by default because it requires restartPods to be enabled.


Yeah I don't think we need this todo here. We expect this and aligned that this is expected. We aren't going to have "parity" by clearing all annotations when restartPods is false.

main.go

lisguo · 2025-05-08T14:24:17Z

main.go

+	if os.Getenv("DISABLE_AUTO_ANNOTATION") == "true" {
+		setupLog.Info("Auto-annotation is disabled")
+	} else if autoAnnotationConfigStr != "" {
+		if err := json.Unmarshal([]byte(autoAnnotationConfigStr), &autoAnnotationConfig); err != nil {


if an auto annotation config exists, and we are unable to marshal shouldn't we just return the error here? Why keep going?

because we might want to use AutoMonitor instead

i could go either way on this

yeah I'm thinking if a customer has an auto annotation config -- the intention is to use auto annotation and if that fails we should just return an error. But not a big deal

pkg/instrumentation/auto/callback.go

pkg/instrumentation/auto/monitor.go

…onitor

…il.go in auto package. Add unit tests

lisguo · 2025-05-08T18:09:19Z

main.go

+	if os.Getenv("DISABLE_AUTO_ANNOTATION") == "true" {
+		setupLog.Info("Auto-annotation is disabled")
+	} else if autoAnnotationConfigStr != "" {
+		if err := json.Unmarshal([]byte(autoAnnotationConfigStr), &autoAnnotationConfig); err != nil {


yeah I'm thinking if a customer has an auto annotation config -- the intention is to use auto annotation and if that fails we should just return an error. But not a big deal

pkg/instrumentation/auto/util.go

jefchien · 2025-05-08T21:17:13Z

pkg/instrumentation/auto/util_test.go

+				assert.NotNil(t, annotator, "Expected non-nil annotator")
+
+				// Check type using type assertion
+				actualType := fmt.Sprintf("%T", annotator)


nit: Would prefer to use reflection or some enum that checks the actual type instead of these string representations. If we ever rename the struct or package or move it to a different package, these tests will break.

Could do something like

expectedType: reflect.TypeOf(auto.AnnotationMutators{}) ... assert.Equals(t, tt.expectedType, reflect.TypeOf(annotator))

or

type annotatorType int const ( unset annotatorType = iota mutators monitor ) ... switch annotator.(type) { case *auto.AnnotationMutators: assert.Equals(t, mutators, tt.expectedType) }

jefchien · 2025-05-08T21:21:55Z

pkg/instrumentation/auto/util.go

+	}
+
+	if autoAnnotationConfigStr == "" {
+		return nil, fmt.Errorf("auto-annotation configuration not provided, disabling AutoAnnotation")


Why does this get logged as an error when the environment variable case is logged as an info message? This seems like a common case.

Agree that this probably doesnt need to be an error.
But dont think this will be very common since our add-on/chart will set the default for this as the java: {deployments:[], ...} string.

Actually we do the same below for autoAnnotationConfig.Empty() and that will be true.

jefchien · 2025-05-08T21:23:51Z

pkg/instrumentation/auto/config.go

+	return typesSelected
+}
+
+func (c AnnotationConfig) Empty() bool {


super minor nit: Naming might suggest that it would empty the configuration.

Suggested change

func (c AnnotationConfig) Empty() bool {

func (c AnnotationConfig) IsEmpty() bool {

the-mann · 2025-05-09T13:56:29Z

/

the-mann added 17 commits March 10, 2025 10:06

almost all passing

7c8774d

passing

95b11c2

check pod template labels instead of workload labels

2a6d9df

Provide Monitor functions (except Mutate) workload instead of pod tem…

b50163c

…plate ObjectMeta

big progress

3ce94cf

more progress

97ee02b

works

24c29cd

blah

b2d6c1c

blah

c70f7f8

fix logging, add more testing

e47e85a

remove unnecessary HasSynced

6ab7980

Handle service deletion, service update. test service deletion

82750e1

Improve backwards compatability, make MutateAndPatchAll work with bot…

a886b9a

…h AutoMonitor and AnnotationMutators, check if safe to mutate before applying custom selector, run opt out tests for each type of workload, test opt out by disabling monitorallservices

Maintain legacy autoAnnotateAutoInstrumentation behavior.

9d6b4a7

CustomSelectors should obey autoRestart, autoAnnotateAutoInstrumentation shouldn't. Rename MonitorInterface to InstrumentationAnnotator. Add docs.

Clean up

1216808

undo dockerfile update

b5a0e96

Fix integration tests

e1cf759

sky333999 commented Mar 26, 2025

View reviewed changes

main.go Outdated Show resolved Hide resolved

the-mann added 12 commits March 26, 2025 16:02

Rename auto_monitor*.go files to monitor*.go, rename AutoRestart to R…

33df3e8

…estartPods

Enable multi instrumentation feature flag, enable skip multi instrume…

734966f

…ntation container validation by default. This is required for automonitor to work, because it tries to apply all init containers to each selected pod.

come up with actual commit message

ed696e5

Update tests to expect namespace in exclude and customSelector to…

319e18a

… update pod templates in workloads.

add transformer, warning function

42f6e23

fix integration tests to work with new exclude syntax

d35ed59

namespace-level exclude should update pod template, but namespace-lev…

fbafe30

…el custom selector should not update pod template

Use correct autoAnnotationConfigStr to create Instrumentation Annotator

d65c9f0

Restart namespace for AutoMonitor based on restartPods, always rest…

eecdc14

…art namespace for autoAnnotateAutoInstrumentation.

omit languages, exclude, and customselector when empty

185ff81

add more integ tests for AutoMonitor

c28093a

fix permutation 9 (don't use kube-system)

707856b

the-mann added 5 commits May 7, 2025 10:56

Revert Dockerfile change

5552674

Implement feedback

9c8bc8d

Remove integ tests

b68d141

Remove integ tests

8667214

go format

3fb6a0f

the-mann marked this pull request as ready for review May 7, 2025 20:51

jefchien reviewed May 8, 2025

View reviewed changes

integration-tests/util/util.go Outdated Show resolved Hide resolved

integration-tests/util/util.go Outdated Show resolved Hide resolved

pkg/instrumentation/annotationtype.go Show resolved Hide resolved

pkg/instrumentation/annotationtype.go Outdated Show resolved Hide resolved

finish rolling back integration test changes

910a042

jefchien reviewed May 8, 2025

View reviewed changes

pkg/instrumentation/auto/callback.go Outdated Show resolved Hide resolved

pkg/instrumentation/auto/config.go Show resolved Hide resolved

pkg/instrumentation/auto/monitor.go Outdated Show resolved Hide resolved

pkg/instrumentation/podmutator.go Outdated Show resolved Hide resolved

lisguo reviewed May 8, 2025

View reviewed changes

the-mann added 7 commits May 8, 2025 13:39

SupportedTypes() -> SupportedTypes

2f987de

update docs

ba8063c

inline function for shouldOverrideMultiInstrumentation

9c6ce5a

name of variable should be same as struct name

b145244

Remove todo, only check if autoAnnotationConfig is empty to use AutoM…

3ee911c

…onitor

move excludedNamespaces to top

ddfd552

Refactor InstrumentationAnnotator creation out of main.go and into ut…

0e27206

…il.go in auto package. Add unit tests

the-mann force-pushed the mpmann/appsignals-1-step branch from ef66e90 to 0e27206 Compare May 8, 2025 19:42

lisguo previously approved these changes May 8, 2025

View reviewed changes

Incorporate auto/util.go feedback

fe01d21

the-mann dismissed lisguo’s stale review via fe01d21 May 8, 2025 20:12

lisguo approved these changes May 8, 2025

View reviewed changes

jefchien reviewed May 8, 2025

View reviewed changes

jefchien approved these changes May 8, 2025

View reviewed changes

sky333999 changed the title ~~AutoMonitor~~ Introduce AutoMonitor for simpler enablement of Application Signals May 8, 2025

the-mann closed this May 9, 2025

the-mann mentioned this pull request May 9, 2025

Introduce AutoMonitor for simpler enablement of Application Signals #309

Merged

sky333999 deleted the mpmann/appsignals-1-step branch May 9, 2025 14:18

the-mann mentioned this pull request May 15, 2025

Auto Monitor follow up changes #311

Merged

	func (c AnnotationConfig) Empty() bool {
	func (c AnnotationConfig) IsEmpty() bool {

Introduce AutoMonitor for simpler enablement of Application Signals #297

Introduce AutoMonitor for simpler enablement of Application Signals #297

Uh oh!

Conversation

sky333999 commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-mann commented May 9, 2025

Uh oh!

Uh oh!

sky333999 commented Mar 24, 2025 •

edited

Loading